From 2dea830ecce09c2de5f5186fd667e607ad9da7b5 Mon Sep 17 00:00:00 2001 From: Moriel Schottlender Date: Mon, 15 May 2017 14:43:33 -0700 Subject: [PATCH] RCFilters: Minimize url string In order to minimize the URL query, we use a base representation of the parameters as if they were all '0' or '' and internally expand on it. - Only display parameters with a value that is not empty or '0' in the URI. Any parameter that is missing from the URI is presumed to have an empty value. - Stop pushing defaultParameters everywhere. Default parameters should only be considered either on load (when/if needed) or when the user actively requests for them. - Minimize parameters to the URL, and expand when reading into the model. Similar to using base filters, we can use a representation of base parameters to make the URL small but the representation all-encompassing. Bug: T165445 Change-Id: I1d21c38137fde51fcd561e2de24592722bf532c6 --- .../dm/mw.rcfilters.dm.FiltersViewModel.js | 10 +- .../mw.rcfilters.Controller.js | 149 ++++++++++++++---- .../dm.FiltersViewModel.test.js | 27 +++- 3 files changed, 149 insertions(+), 37 deletions(-) diff --git a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js index 88ce33c19a..402dbf928c 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js @@ -405,10 +405,14 @@ mw.rcfilters.dm.FiltersViewModel.prototype.getDefaultParams = function () { var result = {}; + // Get default filter state $.each( this.groups, function ( name, model ) { result = $.extend( true, {}, result, model.getDefaultParams() ); } ); + // Get default highlight state + result = $.extend( true, {}, result, this.getHighlightParameters() ); + return result; }; @@ -505,11 +509,13 @@ * are the selected highlight colors. */ mw.rcfilters.dm.FiltersViewModel.prototype.getHighlightParameters = function () { - var result = { highlight: Number( this.isHighlightEnabled() ) }; + var result = {}; this.getItems().forEach( function ( filterItem ) { - result[ filterItem.getName() + '_color' ] = filterItem.getHighlightColor(); + result[ filterItem.getName() + '_color' ] = filterItem.getHighlightColor() || null; } ); + result.highlight = String( Number( this.isHighlightEnabled() ) ); + return result; }; diff --git a/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js b/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js index e9274f53b9..aec2922bcb 100644 --- a/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js +++ b/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js @@ -12,7 +12,8 @@ this.changesListModel = changesListModel; this.savedQueriesModel = savedQueriesModel; this.requestCounter = 0; - this.baseState = {}; + this.baseFilterState = {}; + this.emptyParameterState = {}; }; /* Initialization */ @@ -24,12 +25,22 @@ * @param {Array} filterStructure Filter definition and structure for the model */ mw.rcfilters.Controller.prototype.initialize = function ( filterStructure ) { - var parsedSavedQueries, + var parsedSavedQueries, validParameterNames, + uri = new mw.Uri(), $changesList = $( '.mw-changeslist' ).first().contents(); + // Initialize the model this.filtersModel.initializeFilters( filterStructure ); this._buildBaseFilterState(); + this._buildEmptyParameterState(); + validParameterNames = Object.keys( this._getEmptyParameterState() ) + .filter( function ( param ) { + // Remove 'highlight' parameter from this check; + // if it's the only parameter in the URL we still + // want to consider the URL 'empty' for defaults to load + return param !== 'highlight'; + } ); try { parsedSavedQueries = JSON.parse( mw.user.options.get( 'rcfilters-saved-queries' ) || '{}' ); @@ -42,10 +53,40 @@ // can normalize them per each query item this.savedQueriesModel.initialize( parsedSavedQueries, - this._getBaseState() + this._getBaseFilterState() ); - this.updateStateBasedOnUrl(); + // Check whether we need to load defaults. + // We do this by checking whether the current URI query + // contains any parameters recognized by the system. + // If it does, we load the given state. + // If it doesn't, we have no values at all, and we assume + // the user loads the base-page and we load defaults. + // Defaults should only be applied on load (if necessary) + // or on request + if ( + Object.keys( uri.query ).some( function ( parameter ) { + return validParameterNames.indexOf( parameter ) > -1; + } ) + ) { + // There are parameters in the url, update model state + this.updateStateBasedOnUrl(); + } else { + // No valid parameters are given, load defaults + this._updateModelState( + $.extend( + true, + // We've ignored the highlight parameter for the sake + // of seeing whether we need to apply defaults - but + // while we do load the defaults, we still want to retain + // the actual value given in the URL for it on top of the + // defaults + { highlight: String( Number( uri.query.highlight ) ) }, + this._getDefaultParams() + ) + ); + this.updateChangesList(); + } // Update the changes list with the existing data // so it gets processed @@ -59,7 +100,7 @@ * Reset to default filters */ mw.rcfilters.Controller.prototype.resetToDefaults = function () { - this._updateModelState( this._getDefaultParams() ); + this._updateModelState( $.extend( true, { highlight: '0' }, this._getDefaultParams() ) ); this.updateChangesList(); }; @@ -335,15 +376,33 @@ } ); highlightedItems.highlight = false; - this.baseState = { + this.baseFilterState = { filters: this.filtersModel.getFiltersFromParameters( defaultParams ), highlights: highlightedItems }; }; /** - * Get an object representing the base state of parameters - * and highlights. The structure is similar to what we use + * Build an empty representation of the parameters, where all parameters + * are either set to '0' or '' depending on their type. + * This must run during initialization, before highlights are set. + */ + mw.rcfilters.Controller.prototype._buildEmptyParameterState = function () { + var emptyParams = this.filtersModel.getParametersFromFilters( {} ), + emptyHighlights = this.filtersModel.getHighlightParameters(); + + this.emptyParameterState = $.extend( + true, + {}, + emptyParams, + emptyHighlights, + { highlight: '0' } + ); + }; + + /** + * Get an object representing the base filter state of both + * filters and highlights. The structure is similar to what we use * to store each query in the saved queries object: * { * filters: { @@ -357,8 +416,24 @@ * @return {Object} Object representing the base state of * parameters and highlights */ - mw.rcfilters.Controller.prototype._getBaseState = function () { - return this.baseState; + mw.rcfilters.Controller.prototype._getBaseFilterState = function () { + return this.baseFilterState; + }; + + /** + * Get an object representing the base state of parameters + * and highlights. The structure is similar to what we use + * to store each query in the saved queries object: + * { + * param1: "value", + * param2: "value1|value2" + * } + * + * @return {Object} Object representing the base state of + * parameters and highlights + */ + mw.rcfilters.Controller.prototype._getEmptyParameterState = function () { + return this.emptyParameterState; }; /** @@ -374,7 +449,7 @@ */ mw.rcfilters.Controller.prototype._getMinimalFilterList = function ( valuesObject ) { var result = { filters: {}, highlights: {} }, - baseState = this._getBaseState(); + baseState = this._getBaseFilterState(); // XOR results $.each( valuesObject.filters, function ( name, value ) { @@ -434,13 +509,12 @@ /** * Update filter state (selection and highlighting) based - * on current URL and default values. + * on current URL values. */ mw.rcfilters.Controller.prototype.updateStateBasedOnUrl = function () { - var uri = new mw.Uri(), - defaultParams = this._getDefaultParams(); + var uri = new mw.Uri(); - this._updateModelState( $.extend( {}, defaultParams, uri.query ) ); + this._updateModelState( uri.query ); this.updateChangesList(); }; @@ -566,26 +640,35 @@ */ mw.rcfilters.Controller.prototype._getUpdatedUri = function () { var uri = new mw.Uri(), - highlightParams = this.filtersModel.getHighlightParameters(); - - // Add to existing queries in URL - // TODO: Clean up the list of filters; perhaps 'falsy' filters - // shouldn't appear at all? Or compare to existing query string - // and see if current state of a specific filter is needed? - uri.extend( this.filtersModel.getParametersFromFilters() ); + highlightParams = this.filtersModel.getHighlightParameters(), + modelParameters = this.filtersModel.getParametersFromFilters(), + baseParams = this._getEmptyParameterState(); + + // Minimize values of the model parameters; show only the values that + // are non-zero. We assume that all parameters that are not literally + // showing in the URL are set to zero or empty + $.each( modelParameters, function ( paramName, value ) { + if ( baseParams[ paramName ] !== value ) { + uri.query[ paramName ] = value; + } else { + // We need to remove this value from the url + delete uri.query[ paramName ]; + } + } ); // highlight params - uri.query.highlight = Number( this.filtersModel.isHighlightEnabled() ); - Object.keys( highlightParams ).forEach( function ( paramName ) { - // Always have some value (either the color or null) so that - // if we have something in the URL that doesn't have the highlight - // intentionally, it can override default with highlight. - // Otherwise, the $.extend will always add the highlight that - // exists in the default even if the URL query that is being - // refreshed has different highlights, or has highlights enabled - // but no active highlights anywhere - uri.query[ paramName ] = highlightParams[ paramName ] ? - highlightParams[ paramName ] : null; + if ( this.filtersModel.isHighlightEnabled() ) { + uri.query.highlight = Number( this.filtersModel.isHighlightEnabled() ); + } else { + delete uri.query.highlight; + } + $.each( highlightParams, function ( paramName, value ) { + // Only output if it is different than the base parameters + if ( baseParams[ paramName ] !== value ) { + uri.query[ paramName ] = value; + } else { + delete uri.query[ paramName ]; + } } ); return uri; diff --git a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js index 618afb2df8..714739b680 100644 --- a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js +++ b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js @@ -188,6 +188,16 @@ assert.deepEqual( model.getDefaultParams(), { + group1__hidefilter1_color: null, + group1__hidefilter2_color: null, + group1__hidefilter3_color: null, + group2__hidefilter4_color: null, + group2__hidefilter5_color: null, + group2__hidefilter6_color: null, + group3__filter7_color: null, + group3__filter8_color: null, + group3__filter9_color: null, + highlight: '0', hidefilter1: '1', hidefilter2: '0', hidefilter3: '1', @@ -398,7 +408,7 @@ hidefilter6: '0', group3: '' }, - 'One filters in one "send_unselected_if_any" group returns the other parameters truthy.' + 'One filter in one "send_unselected_if_any" group returns the other parameters truthy.' ); // Select 2 filters @@ -587,7 +597,7 @@ }, { // This is mocking case above - // - 'One filters in one "send_unselected_if_any" group returns the other parameters truthy.' + // - 'One filter in one "send_unselected_if_any" group returns the other parameters truthy.' input: { group1__hidefilter1: 1 }, @@ -603,6 +613,19 @@ group3: '' }, msg: 'Given an explicit (incomplete) filter state object, the result is the same as if the object give represented the model state.' + }, + { + input: {}, + expected: { + hidefilter1: '0', + hidefilter2: '0', + hidefilter3: '0', + hidefilter4: '0', + hidefilter5: '0', + hidefilter6: '0', + group3: '' + }, + msg: 'Given an explicit empty object, the result is all filters set to their falsey unselected value.' } ]; -- 2.20.1